Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Envoy proxy failing in tcp-metrics #49

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

acald-creator
Copy link
Contributor

@acald-creator acald-creator commented Sep 3, 2022

Description

This PR Fixes #39

Updated envoy.yaml

  • Envoy now calls for typed_config in v3
  • Rearranged placement of the filters, but it probably can be reworked to make it cleaner
  • Clusters is now set up with an loadbalancer endpoint, instead of hosts
  • Probably need to fix spacing/formatting
  • Updated proxy-wasm from v0.1.0 to v0.2.0
  • Other envoy.yaml will need to be updated, currently working on the rest and will add to this PR.

Signed-off-by: Antonette Caldwell [email protected]

Notes for Reviewers

Signed commits

  • [ x ] Yes, I signed my commits.

@welcome
Copy link

welcome bot commented Sep 3, 2022

Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, please review the Layer5 Contributors' Welcome Guide and sure to join the community Slack.
Be sure to double-check that you have signed your commits. Here are instructions for making signing an implicit activity while peforming a commit.

Description: This fixes a previous open issue layer5io#39

Updated envoy.yaml
- Envoy now calls for typed_config in `v3`
- Rearranged placement of the filters, but it probably can be reworked to make it cleaner
- Clusters is now set up with an endpoint, instead of `hosts`
- Probably need to fix spacing

Signed-off-by: Antonette Caldwell <[email protected]>
@leecalcote
Copy link
Member

@felixfaisal you're welcome to offer review...

@felixfaisal
Copy link
Contributor

Thanks @leecalcote
LGTM, the problem was with the deprecation of some yml syntax in v3 and this PR is updating it accordingly so looks good.

@acald-creator
Copy link
Contributor Author

@felixfaisal Thanks!

This PR will have more updates very soon. I just have the JWT to finish up and then this PR will be updated.

@acald-creator
Copy link
Contributor Author

@felixfaisal Is this PR sufficient at this time?

Sorry, time got away from me, and I'm circling back now to finish the rest of the PR.

However, I want to finish the JWT on a separate branch which I have it at here https://github.com/acald-creator/wasm-filters/tree/test-filters.

I wanted to research the information about JWT manipulation since the readme was scarce. But for now, this PR should be go to go unless something is wrong.

If there is something wrong, I can fix it which I probably already did from the other branch.

Copy link

@b0id b0id left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see nothing wrong with @acald-creator and her work here. The issue appears to be corrected and if there are no further problems, her fix seems to have done the trick. @leecalcote

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[Bug] Envoy proxy container failing
4 participants